Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve the performance of
save
and friends
The biggest source of the performance regression in these methods occurred because dirty tracking required eagerly materializing and type casting the assigned values. In the previous commits, I've changed dirty tracking to perform the comparisons lazily. However, all of this is moot when calling `save`, since `changes_applied` will be called, which just ends up eagerly materializing everything, anyway. With the new mutation tracker, it's easy to just compare the previous two hashes in the same lazy fashion. We will not have aliasing issues with this setup, which is proven by the fact that we're able to detect nested mutation. Before: User.create! 2.007k (± 7.1%) i/s - 10.098k After: User.create! 2.557k (± 3.5%) i/s - 12.789k Fixes #19859
- Loading branch information
Showing
3 changed files
with
41 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hello @sgrif 🙇 I think that this method can now be rewritten from:
to:
without losing performance but let me know if that's correct.
Calling
is equivalent to calling
When this commit 136fc65 was made, Active Record had its own
previous_changes
method, added here below. However, that method has been recently removed from the codebase, soprevious_changes
is now only the method defined in Active Model as:Since we are dealing with a memoized Hash, there is probably no need to check
if .include?(attr_name)
before trying to fetch[attr]
for it.Does that make sense? Did I miss anything? Thanks!